Halt upgrades if evr is not owned by foreman on external DB #953
Conversation
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
ianballou
force-pushed
the
halt-upgrade-remote-db-evr-permissions
branch
2 times, most recently
from
4cd6c7a to
a54ac48
Compare
ianballou
force-pushed
the
halt-upgrade-remote-db-evr-permissions
branch
from
a54ac48 to
d4d3d5b
Compare
|
All the comments should be addressed now. |
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
| unless evr_owned_by_postgres.empty? | ||
| return evr_owned_by_postgres.first['evr_owned_by_postgres'] == '0' | ||
| end | ||
| fail!('Could not determine if the evr extension is owned by the foreman DB owner') |
There was a problem hiding this comment.
What do we expect a user that hits this fail statement to do next? Can we give them some direction?
There was a problem hiding this comment.
The only failures reasons I can think of would be if the DB connection failed or if the config doesn't have the correct username information. I've updated the error message.
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
ianballou
force-pushed
the
halt-upgrade-remote-db-evr-permissions
branch
from
40e70cd to
d86def9
Compare
|
I stopped hardcoding 'foreman' as the DB name just in case. We are hardcoding it in the installer check, but I figured why not here since it seems to be supported. |
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
definitions/checks/foreman/check_external_db_evr_permissions.rb
Outdated
Show resolved
Hide resolved
|
I had just a minor nit, @evgeni please also do a final review |
ianballou
force-pushed
the
halt-upgrade-remote-db-evr-permissions
branch
from
d86def9 to
e6dc6f1
Compare
| SQL | ||
| end | ||
|
|
||
| def query_if_postgres_owns_evr |
There was a problem hiding this comment.
Technically, this doesn't check whether postgres owns it, but whether foreman_db_user (not) owns it.
|
|
||
| def foreman_owns_evr? | ||
| evr_owned_by_postgres = feature(:foreman_database).query(query_if_postgres_owns_evr) | ||
| unless evr_owned_by_postgres.empty? |
There was a problem hiding this comment.
I guess it won't ever be empty, given at this point we do know the extension exists, so it must have some owner?
But it also doesn't hurt to play safe here :)
evgeni
changed the title
We (you 😄 ) don't hard code it in the installer check ;) |
Ah I was just thinking of |
It's not the db name either. It's the "config section" aka "whose DB are we looking at" |
Halts upgrades if the
evrextension in Katello external DBs are not owned byforeman. Skips the checks if theevrextension does not exist.Related installer PR: theforeman/foreman-installer#984